Support reading empty datasets and attributes #67
Conversation
| to_array(): string | number | boolean | JSONCompatibleOutputData[]; | ||
| get value(): OutputData | null; | ||
| get json_value(): JSONCompatibleOutputData | null; | ||
| to_array(): JSONCompatibleOutputData | null; |
There was a problem hiding this comment.
Adding | null to OutputData and JSONCompatibleOutputData would have been quite challenging because the types of recursive. I opted for this simpler typing solution, which also avoids dealing with null in deeper internal functions (e.g. process_data).
Note also that I fixed a few type inference issues (string | number | boolean | JSONCompatibleOutputData[] is the same as JSONCompatibleOutputData).
src/hdf5_hl.ts
Outdated
| let known_type = true; | ||
| // let length: number; | ||
| if (type === Module.H5T_class_t.H5T_STRING.value) { | ||
| if (metadata.vlen) { | ||
| let output = []; | ||
| let output: string[] = []; |
There was a problem hiding this comment.
The more recent version of TypeScript in VS Code infers this as never[] and shows an error on the line with output.push() below. There were a few cases like this.
| @@ -153,9 +158,9 @@ function process_data(data: Uint8Array, metadata: Metadata, json_compatible: boo | |||
| else if (type === Module.H5T_class_t.H5T_COMPOUND.value) { | |||
| const { size, compound_type } = <{size: Metadata["size"], compound_type: CompoundTypeMetadata}>metadata; | |||
| let n = Math.floor(data.byteLength / size); | |||
| let output = []; | |||
| let output: (OutputData | JSONCompatibleOutputData)[] = []; | |||
There was a problem hiding this comment.
Not quite accurate (should be OutputData[] | JSONCompatibleOutputData[]) but sufficient to get rid of the error.
| @@ -168,7 +173,7 @@ function process_data(data: Uint8Array, metadata: Metadata, json_compatible: boo | |||
|
|
|||
| else if (type === Module.H5T_class_t.H5T_ARRAY.value) { | |||
| const { array_type } = <{array_type: Metadata}>metadata; | |||
| shape = shape.concat(array_type.shape); | |||
| shape = (<number[]>shape).concat(<number[]>array_type.shape); | |||
There was a problem hiding this comment.
process_data should never be called with a null shape, and the array_type metadata should never be parsed with a null. Could check this with proper runtime assertions, but didn't seem worth it.
| const { json_value, metadata } = this; | ||
| const { shape } = metadata; | ||
| if (!isIterable(json_value) || typeof json_value === "string") { | ||
| return json_value; | ||
| } | ||
| return create_nested_array(json_value, shape); | ||
| return create_nested_array(json_value, <number[]>shape); |
There was a problem hiding this comment.
If the attribute is empty, then json_value is null (i.e. not iterable).
| @@ -529,27 +534,27 @@ export class Attribute { | |||
| this.shape = metadata.shape; | |||
| } | |||
|
|
|||
| get value() { | |||
| get value(): OutputData | null { | |||
There was a problem hiding this comment.
Just being a bit more explicit (here and on a few other methods). It helps double check that the implementation of the method is typed correctly (notably get_attr in this case).
| @@ -19,14 +19,14 @@ export interface H5T_class_t { | |||
|
|
|||
| export interface Metadata { | |||
| array_type?: Metadata, | |||
| chunks: Array<number> | null, | |||
| chunks: number[] | null, | |||
There was a problem hiding this comment.
Minor stylistic change.
|
This is great! Thanks for the PR. I don't have time to merge it right this second but probably later in the week. |
|
Thanks, no rush at all! 😉 |
Currently, when encountering an empty dataset (e.g. with h5py:
grp.create_dataset("empty", dtype=np.float32)) or an empty attribute (e.g. with h5py:grp.attrs["empty"] = h5py.Empty(np.float32)),h5wasmparses the shape as[]and returns the valueundefined.Two problems with this:
undefinedvalue can be limiting - e.g. if you put the value in a state once you've read it, then it's difficult to know when the value has actually been read.With this PR, h5wasm now returns
nullshapes andnullvalues for empty datasets and attributes.